-
-
Notifications
You must be signed in to change notification settings - Fork 611
feat(ui): show reason for failure in endpoint events #1349
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
|
I'm not exactly sold on the look just yet, but I was curious to see what your opinion was, or what your appetite for such a thing would be. As a user I would like to know "why" and "what" failed with such a program, but you may have different intentions. |
|
This isn't a bad idea, but what if there are multiple reasons for the endpoint being unhealthy? For instance, what if, for the first 20 minutes, it's unhealthy because of the status, but from 20 to 40 minutes, it's unhealthy because of the response time? |
|
I'm sure that Health criteria can become quite complex - so if we have an example EndpointA with health criteria: criteria1, criteria2, and criteria3... If it fails at first due to criteria1, then after 10 minutes (or some other time-based criteria requirement) fail due to criteria2, I've seen some platforms have the UI reflect that e.g.: "failed due to criteria1" What do you think of something like that? Or instead, we could show just all criteria that failed initially (or after X amount of time) and leave it to the user's discretion :) |
|
Perhaps having it as a set (unique values) would be sufficient for now. Just a heads up, the current implementation is missing a lot of bits and pieces, the biggest one being persistence. |
| FailedConditionReasons []FailureReason `json:"failedConditionReasons,omitempty"` | ||
| } | ||
|
|
||
| // EventType is, uh, the types of events? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lmao, I forgot about this
This is a great idea, but there's a few things we'll have to take into consideration, such as everything under If a user configures it so certain things are hidden, they also shouldn't be reflected in the event, as it may be considered sensitive to the user. Also, I want to point out that I'm working on replacing the chart as we speak, and I'm thinking of perhaps incorporating events (specifically failed events) in the chart to show when the service is unhealthy, so whatever we add in this PR could also be included as information in the chart (i.e. show the failed reasons on hover) |

Summary
This PR shows the reason as to why an Endpoint failed, instead of just "it failed". E.g. right now you look at something like this:

and you ask yourself "But why did it fail? What criteria failed?"
So this is what I've designed at this point:

Checklist
README.md, if applicable.